gh-142349: Fix refcount corruption in lazy import specialization#144733
gh-142349: Fix refcount corruption in lazy import specialization#144733Yhg1s merged 1 commit intopython:mainfrom
Conversation
pablogsal
requested review from
kumaraditya303,
ncoghlan and
warsaw
as code owners
pablogsal
changed the title
pablogsal
force-pushed
the
lazy-cleanup-2
branch
from
3007570 to
239c717
Compare
Yhg1s
left a comment
Yhg1s
left a comment
There was a problem hiding this comment.
I don't think this is good enough for a free-threaded build. The error case happens when _PyImport_LoadLazyImportTstate is called with a borrowed reference. The GIL won't be protecting that borrowed reference in a free-threaded build, so the INCREF already happens too late. We need the caller to acquire an owned reference instead, in a safe way. But... where does this happen? As far as I can tell the callers all own the reference they pass.
If that were true then there would not be crashes no? |
Unless the problem is actually something different, yes :) I would like to know which it is. |
pablogsal
force-pushed
the
lazy-cleanup-2
branch
from
ca6f0f9 to
2fe3763
Compare
_PyDict_LookupIndexAndValue() returns a borrowed reference via _Py_dict_lookup(), but specialize_load_global_lock_held() called Py_DECREF(value) on it when bailing out for lazy imports. Each time the adaptive counter fired while a lazy import was still in globals, this stole one reference from the dict's object. With 8+ threads racing through LOAD_GLOBAL during concurrent lazy import resolution, enough triggers accumulated to drive the refcount to zero while the dict and other threads still referenced the object, causing use-after-free.
pablogsal
force-pushed
the
lazy-cleanup-2
branch
from
2fe3763 to
c7defe2
Compare
Narrator: and the problem was indeed something different. Kind of |
3 participants
_PyDict_LookupIndexAndValue() returns a borrowed reference via _Py_dict_lookup(), but specialize_load_global_lock_held() called Py_DECREF(value) on it when bailing out for lazy imports. Each time the adaptive counter fired while a lazy import was still in globals, this stole one reference from the dict's object. With 8+ threads racing through LOAD_GLOBAL during concurrent lazy import resolution, enough triggers accumulated to drive the refcount to zero while the dict and other threads still referenced the object, causing use-after-free.